fix: arg parsing and exiting, more returns#3
Conversation
📝 WalkthroughWalkthroughModified main.cpp's argument handling to skip the program name (argv[0]) when processing multiple arguments. Enhanced help flag detection to check argv[1] specifically, with immediate program exit after help output. Added unconditional return statement after --pid handling for explicit control flow termination. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai configuration |
|
Configuration used: defaults CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
inheritance: false
reviews:
profile: chill
request_changes_workflow: false
high_level_summary: true
high_level_summary_instructions: ''
high_level_summary_placeholder: '@coderabbitai summary'
high_level_summary_in_walkthrough: false
auto_title_placeholder: '@coderabbitai'
auto_title_instructions: ''
review_status: true
commit_status: true
fail_commit_status: false
collapse_walkthrough: true
changed_files_summary: true
sequence_diagrams: true
estimate_code_review_effort: true
assess_linked_issues: true
related_issues: true
related_prs: true
suggested_labels: true
labeling_instructions: []
auto_apply_labels: false
suggested_reviewers: true
auto_assign_reviewers: false
in_progress_fortune: true
poem: true
enable_prompt_for_ai_agents: true
path_filters: []
path_instructions: []
abort_on_close: true
disable_cache: false
auto_review:
enabled: true
auto_incremental_review: true
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
ignore_usernames: []
finishing_touches:
docstrings:
enabled: true
unit_tests:
enabled: true
pre_merge_checks:
docstrings:
mode: warning
threshold: 80
title:
mode: warning
requirements: ''
description:
mode: warning
issue_assessment:
mode: warning
custom_checks: []
tools:
ast-grep:
rule_dirs: []
util_dirs: []
essential_rules: true
packages: []
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_rules: []
disabled_rules: []
enabled_categories: []
disabled_categories: []
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
phpmd:
enabled: true
phpcs:
enabled: true
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
checkov:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
flake8:
enabled: true
fortitudeLint:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
clang:
enabled: true
cppcheck:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
clippy:
enabled: true
sqlfluff:
enabled: true
prismaLint:
enabled: true
pylint:
enabled: true
oxc:
enabled: true
shopifyThemeCheck:
enabled: true
luacheck:
enabled: true
brakeman:
enabled: true
dotenvLint:
enabled: true
htmlhint:
enabled: true
checkmake:
enabled: true
osvScanner:
enabled: true
chat:
art: true
auto_reply: true
integrations:
jira:
usage: auto
linear:
usage: auto
knowledge_base:
opt_out: false
web_search:
enabled: true
code_guidelines:
enabled: true
filePatterns: []
learnings:
scope: auto
issues:
scope: auto
jira:
usage: auto
project_keys: []
linear:
usage: auto
team_keys: []
pull_requests:
scope: auto
mcp:
usage: auto
disabled_servers: []
code_generation:
docstrings:
language: en-US
path_instructions: []
unit_tests:
path_instructions: []
issue_enrichment:
auto_enrich:
enabled: false
planning:
enabled: true
auto_planning:
enabled: true
labels: []
labeling:
labeling_instructions: []
auto_apply_labels: false
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.cpp (1)
274-312: Refactor: Move help check outside the argument loop.The help check is inside the argument processing loop but always examines
argv[1]rather than the current argument. This means:
- The help check executes on every loop iteration, which is wasteful
- The logic is confusing: why check
argv[1]inside a loop that iterates over all arguments?- When
argc == 1, the conditionargc == 1protects against out-of-bounds access toargv[1], but this is fragile🔎 Proposed refactor
Move the help check before the loop:
int main(int argc, char* argv[]) { + // Check for help first + if (argc == 1 || (argc > 1 && (std::string(argv[1]) == "-h" || std::string(argv[1]) == "--help"))) { + if (!forkAuthor.empty()) { + std::cout << "\nwin-witr - Why is this running? Windows version by supervoidcoder. Fork by " << forkAuthor << std::endl; + } else { + std::cout << "\nwin-witr - Why is this running? Windows version by supervoidcoder." << std::endl; + } + + if (IsVirtualTerminalModeEnabled()) { + if (IsProcessElevated()) { + std::cout << "\033[1;32mRunning with elevated privileges (Admin).\033[0m" << std::endl; + } else { + std::cout << "\033[1;91mNot running with elevated privileges. Some information may be inaccessible.\033[0m" << std::endl; + } + std::cout << "\033[1;32mUsage:\033[0m win-witr [options]" << std::endl; + std::cout << "\033[1;32mOptions:\033[0m" << std::endl; + std::cout << " \033[1;33m-h, --help\033[0m Show this help message and exit" << std::endl; + std::cout << " \033[1;33m-v, --version\033[0m Show version information and exit" << std::endl; + std::cout << " \033[1;33m--port <port>\033[0m Specify the port to check" << std::endl; + std::cout << " \033[1;33m--pid <pid>\033[0m Specify the PID to check" << std::endl; + std::cout << " \033[1;33m <name>\033[0m Specify the process name to check" << std::endl; + } else { + if (IsProcessElevated()) { + std::cout << "Running with elevated privileges (Admin)." << std::endl; + } else { + std::cout << "Not running with elevated privileges. Some information may be inaccessible." << std::endl; + } + std::cout << "Usage: win-witr [options]" << std::endl; + std::cout << "Options:" << std::endl; + std::cout << " -h, --help Show this help message and exit" << std::endl; + std::cout << " -v, --version Show version information and exit" << std::endl; + std::cout << " --port <port> Specify the port to check" << std::endl; + std::cout << " --pid <pid> Specify the PID to check" << std::endl; + std::cout << " <name> Specify the process name to check" << std::endl; + } + return 0; + } + for (int i = 0; i < argc; ++i) { std::string arg = argv[i]; if (i == 0 && argc > 1) { continue; } - if (argc == 1 || std::string(argv[1]) == "-h" || std::string(argv[1]) == "--help") { - if (!forkAuthor.empty()) { - std::cout << "\nwin-witr - Why is this running? Windows version by supervoidcoder. Fork by " << forkAuthor << std::endl; - } else { - std::cout << "\nwin-witr - Why is this running? Windows version by supervoidcoder." << std::endl; - } - - if (IsVirtualTerminalModeEnabled()) { - if (IsProcessElevated()) { - std::cout << "\033[1;32mRunning with elevated privileges (Admin).\033[0m" << std::endl; - } else { - std::cout << "\033[1;91mNot running with elevated privileges. Some information may be inaccessible.\033[0m" << std::endl; - } - std::cout << "\033[1;32mUsage:\033[0m win-witr [options]" << std::endl; - std::cout << "\033[1;32mOptions:\033[0m" << std::endl; - std::cout << " \033[1;33m-h, --help\033[0m Show this help message and exit" << std::endl; - std::cout << " \033[1;33m-v, --version\033[0m Show version information and exit" << std::endl; - std::cout << " \033[1;33m--port <port>\033[0m Specify the port to check" << std::endl; - std::cout << " \033[1;33m--pid <pid>\033[0m Specify the PID to check" << std::endl; - std::cout << " \033[1;33m <name>\033[0m Specify the process name to check" << std::endl; - - } else { - if (IsProcessElevated()) { - std::cout << "Running with elevated privileges (Admin)." << std::endl; - } else { - std::cout << "Not running with elevated privileges. Some information may be inaccessible." << std::endl; - } - std::cout << "Usage: win-witr [options]" << std::endl; - std::cout << "Options:" << std::endl; - std::cout << " -h, --help Show this help message and exit" << std::endl; - std::cout << " -v, --version Show version information and exit" << std::endl; - std::cout << " --port <port> Specify the port to check" << std::endl; - std::cout << " --pid <pid> Specify the PID to check" << std::endl; - std::cout << " <name> Specify the process name to check" << std::endl; - - - } - return 0; // exit after printing help because it might try to process -help as a process name otherwise - } - if (arg == "-v" || arg == "--version") {This makes the control flow clearer and more efficient.
🧹 Nitpick comments (1)
main.cpp (1)
263-269: Refactor: Start loop ati = 1instead of usingcontinue.The current approach sets
arg = argv[i]on every iteration includingi=0, then immediately skips processing wheni==0. This is inefficient and creates unnecessary confusion.🔎 Proposed refactor
- for (int i = 0; i < argc; ++i) { + for (int i = 1; i < argc; ++i) { std::string arg = argv[i]; - - - if (i == 0 && argc > 1) { - continue; - } - - -This makes the intent clearer: process only the actual arguments, not the program name.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
main.exeis excluded by!**/*.exemain.objis excluded by!**/*.obj
📒 Files selected for processing (2)
main.cppvc140.pdb
🧰 Additional context used
🪛 GitHub Actions: Lint
main.cpp
[error] 1-1: clang-format violations detected. code should be clang-formatted [-Wclang-format-violations].
🔇 Additional comments (1)
main.cpp (1)
367-367: Verify: Return after--pidprevents processing subsequent arguments.The unconditional
return 0at Line 367 ensures the program exits immediately after processing a PID. This means commands likewin-witr --pid 1234 notepad.exewill only process the PID and ignorenotepad.exe.Is this intentional behavior (one query per invocation), or should the tool support processing multiple arguments in sequence?
|
@coderabbitai please paste your settings dump here with changes: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@supervoidcoder I'll fetch the latest configuration schema to ensure I provide you with the correct settings for enabling PR approvals. 🛠 Debug Information Gathered @supervoidcoder Here's your configuration with the requested changes: language: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
inheritance: false
reviews:
profile: chill
request_changes_workflow: true # Changed: Enable PR approvals
high_level_summary: true
high_level_summary_instructions: ''
high_level_summary_placeholder: '@coderabbitai summary'
high_level_summary_in_walkthrough: false
auto_title_placeholder: '@coderabbitai'
auto_title_instructions: ''
review_status: true
commit_status: true
fail_commit_status: false
collapse_walkthrough: false # Changed: Don't collapse walkthrough
changed_files_summary: true
sequence_diagrams: true
estimate_code_review_effort: true
assess_linked_issues: true
related_issues: true
related_prs: true
suggested_labels: true
labeling_instructions: []
auto_apply_labels: false
suggested_reviewers: true
auto_assign_reviewers: false
in_progress_fortune: true
poem: true
enable_prompt_for_ai_agents: true
path_filters: []
path_instructions: []
abort_on_close: true
disable_cache: false
auto_review:
enabled: true
auto_incremental_review: true
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
ignore_usernames: []
finishing_touches:
docstrings:
enabled: true
unit_tests:
enabled: true
pre_merge_checks:
docstrings:
mode: warning
threshold: 80
title:
mode: warning
requirements: ''
description:
mode: warning
issue_assessment:
mode: warning
custom_checks: []
tools:
ast-grep:
rule_dirs: []
util_dirs: []
essential_rules: true
packages: []
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_rules: []
disabled_rules: []
enabled_categories: []
disabled_categories: []
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
phpmd:
enabled: true
phpcs:
enabled: true
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
checkov:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
flake8:
enabled: true
fortitudeLint:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
clang:
enabled: true
cppcheck:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
clippy:
enabled: true
sqlfluff:
enabled: true
prismaLint:
enabled: true
pylint:
enabled: true
oxc:
enabled: true
shopifyThemeCheck:
enabled: true
luacheck:
enabled: true
brakeman:
enabled: true
dotenvLint:
enabled: true
htmlhint:
enabled: true
checkmake:
enabled: true
osvScanner:
enabled: true
chat:
art: true
auto_reply: true
integrations:
jira:
usage: auto
linear:
usage: auto
knowledge_base:
opt_out: false
web_search:
enabled: true
code_guidelines:
enabled: true
filePatterns: []
learnings:
scope: auto
issues:
scope: auto
jira:
usage: auto
project_keys: []
linear:
usage: auto
team_keys: []
pull_requests:
scope: auto
mcp:
usage: auto
disabled_servers: []
code_generation:
docstrings:
language: en-US
path_instructions: []
unit_tests:
path_instructions: []
issue_enrichment:
auto_enrich:
enabled: false
planning:
enabled: true
auto_planning:
enabled: true
labels: []
labeling:
labeling_instructions: []
auto_apply_labels: falseChanges made:
You can save this as |
|
@coderabbitai can btw you give me an updated clang lint thing that DOESN'T fail on errors? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@supervoidcoder, I'll fetch the latest configuration schema to see what options are available for the clang tool and provide you with the best configuration. 🏁 Script executed: Length of output: 0 @supervoidcoder, let me fetch the detailed configuration schema to see what options are available for the clang tool. 🛠 Debug Information Gathered @supervoidcoder, I've checked the configuration schema, and unfortunately the The clang tool in CodeRabbit is fairly basic and only performs static analysis checks. Unlike ESLint which provides detailed rule-based feedback, clang's integration is more limited in what it reports. Here's what you can do:
reviews:
tools:
clang:
enabled: false # Disable clang
cppcheck:
enabled: true # Keep cppcheck (already enabled)
You can add this to your reviews:
tools:
clang:
enabled: false
cppcheck:
enabled: falseTry disabling just clang first and see if cppcheck's feedback is more helpful for your workflow. |
|
@coderabbitai no silly I meant the yml GitHub ACCTION |
|
Tip For best results, initiate chat on the files or code changes. @supervoidcoder Ah my apologies! 😅 You're asking about a GitHub Actions workflow for clang linting! Here are a couple of options: Option 1: clang-format (for formatting checks)This won't fail the workflow but will leave comments: name: C++ Linting
on: [pull_request]
jobs:
clang-format:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Run clang-format
uses: jidicula/clang-format-action@v4.11.0
with:
clang-format-version: '17'
check-path: '.'
fallback-style: 'Google'
continue-on-error: true # Won't fail the workflowOption 2: clang-tidy (for static analysis with better feedback)This provides more detailed diagnostic messages: name: C++ Linting
on: [pull_request]
jobs:
clang-tidy:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Install clang-tidy
run: |
sudo apt-get update
sudo apt-get install -y clang-tidy
- name: Run clang-tidy
run: |
clang-tidy src/**/*.cpp -- -std=c++17 || true # || true prevents failure
continue-on-error: trueOption 3: Combined with reviewdog (best feedback)This posts inline comments like ESLint does: name: C++ Linting
on: [pull_request]
jobs:
clang-tidy-review:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: clang-tidy review
uses: ZedThree/clang-tidy-review@v0.16.2
with:
build_dir: .
config_file: ''
continue-on-error: true
- name: Upload review comments
uses: ZedThree/clang-tidy-review/upload@v0.16.2
continue-on-error: trueI'd recommend Option 3 since it gives you inline PR comments similar to ESLint. The 🐰 |
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.